RTL5: allow detach() to just detach immediately if the connection state is suspended/disconnected #411
Conversation
lawrence-forooghian
left a comment
lawrence-forooghian
left a comment
There was a problem hiding this comment.
LGTM — I spent a bit of time thinking about this and I think it makes sense.
The only thing I was unsure about was whether a call to detach() represents a user's request to immediately tear down some potentially-billable resource (i.e. channel attachment) and so whether not waiting to become CONNECTED and send the DETACH means we're not honouring that request, but I don't really know enough about our system to know at what point after becoming disconnected the user stops being billed for the channel resource anyway. Is there anything to worry about here?
…te is suspended/disconnected
SimonWoolf
force-pushed
the
detach-while-disconnceted
branch
from
5f28ef6 to
80b003c
Compare
Nah, waiting until you become connected and sending a detach is a pure no-op in protocol v2 -- the resumed connection is an independent entity to the original and doesn't start out attached to any channels. The original connection will tear itself down after noticing it's disconnected independently of when/whether there's a resume. (and the channelclient and channel coordinator will follow within 2 minutes if no-one else is attached). |
In protocol v2, given we reattach all non-detached channels explicitly on becoming connected, there's no reason that detaching while disconnected, connecting, or suspended shouldn't just immediately work and put the channel into the
detachedstate.also there's no reason to make doing it while
closingresult in an error, all channels are detached on close, it can just succeed. and there's no reason to check for connection==failed, if connection is in a failed state the channel will be also.